feat(experiments): Add saved metrics #25867
Conversation
| { | ||
| "name": "Test Experiment saved metric", | ||
| "description": "Test description", | ||
| "query": {"events": [{"id": "$pageview"}]}, |
There was a problem hiding this comment.
This should only accept ExperimentTrendsQuery | ExperimentFunnelsQuery
(fine to do the validation in a follow up, but maybe better to pass the correct query now to avoid having to update this later)
There was a problem hiding this comment.
oh I see! How do you generally validate this? I could just check the kind in the json, anything else I can do with hogql?
There was a problem hiding this comment.
(not important in this test will do it in a follow up 👍 )
There was a problem hiding this comment.
would something like this work?
if value["kind"] === "ExperimentTrendsQuery":
ExperimentTrendsQuery(**value)
elif value["kind"] === "ExperimentFunnelsQuery":
ExperimentFunnelsQuery(**value)
else:
raise
There was a problem hiding this comment.
ah I see we have pydantic generated schema, should be able to validate that, will do that 👌
There was a problem hiding this comment.
metrics need similar validation right? Not sure where this is happening right now, don't see it in the viewset / serializer
There was a problem hiding this comment.
yep, that would be great!
actually, do we validate filters at all? I don't see it in the serializer
There was a problem hiding this comment.
yeah a little bit, shouldn't have properties and should exist is the only validation.
Problem
Add saved metrics backend, allow adding metadata like metric type to the relation.
TODO: Split this into two PRs, one for models, one for adding model to experiment, otherwise we'll bork things like with holdouts migration.TODO: Add moar tests for experiments handling of saved metrics, and update flow.Changes
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Does this work well for both Cloud and self-hosted?
How did you test this code?